Skip to content

refactor: remove batch score as a pipeline stage#435

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/remove-batch-score
Jul 23, 2026
Merged

refactor: remove batch score as a pipeline stage#435
behinddwalls merged 1 commit into
mainfrom
preetam/remove-batch-score

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

Scoring is no longer a standalone pipeline stage — it is becoming a speculator concern. The dedicated score stage between batch and speculate added a queue hop and a batch state transition (CreatedScored) without owning a distinct decision, so it is redundant. Removing it lets batch hand batches straight to speculate.

What?

The batch controller now publishes to TopicKeySpeculate instead of TopicKeyScore. speculate already accepts BatchStateCreated, so batches flow through without the intermediate Scored transition and speculate itself needs no change.

Removed: the score controller package, TopicKeyScore, the score topic and DLQ registrations, and the orchestrator's scorerFactory wiring. Also removed the batch-level score data — the Batch.Score field, the batch store's UpdateScoreAndState method (no remaining caller), and the score column from the batch schema and queries.

Left intact: the scorer extension (all impls + mock) and the per-queue scorer profiles, retained for the forthcoming scorer-in-speculate integration. The BatchStateScored and RequestStatusScored enums remain — BatchStateScored is still accepted by the untouched speculate controller.

Docs and stale controller-list comments were updated to drop the score stage; scorer-extension references are preserved.

Test Plan

make test — 79/79 unit tests pass (including the retained scorer tests and the reworked mysql batch store test)
bazel build of the orchestrator server — compiles clean, no unused wiring
make gazelle in sync
Not run: make e2e-test (needs Docker). The e2e happy-path assertion was updated to drop RequestStatusScored, since requests no longer pass through the scored status.

@behinddwalls
behinddwalls marked this pull request as ready for review July 23, 2026 19:42
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners July 23, 2026 19:42
@behinddwalls
behinddwalls force-pushed the preetam/remove-batch-score branch from 0c9292b to b0208d3 Compare July 23, 2026 19:52
Scoring is no longer a standalone pipeline stage — it is becoming a speculator concern. The dedicated `score` stage between `batch` and `speculate` added a queue hop and a batch state transition (`Created` → `Scored`) without owning a distinct decision, so it is redundant. Removing it lets `batch` hand batches straight to `speculate`.

The `batch` controller now publishes to `TopicKeySpeculate` instead of `TopicKeyScore`. `speculate` already accepts `BatchStateCreated`, so batches flow through without the intermediate `Scored` transition and speculate itself needs no change.

Removed: the `score` controller package, `TopicKeyScore`, the score topic and DLQ registrations, and the orchestrator's `scorerFactory` wiring. Also removed the batch-level score data — the `Batch.Score` field, the batch store's `UpdateScoreAndState` method (no remaining caller), and the `score` column from the batch schema and queries.

Left intact: the `scorer` extension (all impls + mock) and the per-queue scorer profiles, retained for the forthcoming scorer-in-speculate integration. The `BatchStateScored` and `RequestStatusScored` enums remain — `BatchStateScored` is still accepted by the untouched speculate controller.

Docs and stale controller-list comments were updated to drop the `score` stage; scorer-extension references are preserved.

✅ `make test` — 79/79 unit tests pass (including the retained scorer tests and the reworked mysql batch store test)
✅ `bazel build` of the orchestrator server — compiles clean, no unused wiring
✅ `make gazelle` in sync
Not run: `make e2e-test` (needs Docker). The e2e happy-path assertion was updated to drop `RequestStatusScored`, since requests no longer pass through the scored status.
@behinddwalls
behinddwalls force-pushed the preetam/remove-batch-score branch from b0208d3 to 808bc44 Compare July 23, 2026 21:13
@behinddwalls
behinddwalls merged commit 33f3646 into main Jul 23, 2026
15 checks passed
@behinddwalls
behinddwalls deleted the preetam/remove-batch-score branch July 23, 2026 21:27
behinddwalls added a commit that referenced this pull request Jul 24, 2026
## Summary

### Why?

The dedicated score stage was removed in #435, so nothing transitions a
batch into `BatchStateScored` or emits the `RequestStatusScored`
request-log status anymore. They are now dead, unreachable states — the
speculate controller only kept accepting `BatchStateScored` defensively
for a producer that no longer exists.

### What?

Removed the `BatchStateScored` and `RequestStatusScored` enum values.

- `entity/batch.go`: dropped the const and its entries in
`ActiveBatchStates()` and `DependencyBatchStates()`.
- `entity/request_log.go`: dropped the `RequestStatusScored` const.
- `speculate` controller: `startSpeculation` now switches on
`BatchStateCreated` only (dropped the unreachable `BatchStateScored`
case); doc comment updated to match.
- Tests: dropped the speculate `from_scored` case and switched the
`RequestStatusScored` sample-data usages in the request-log tests to
`RequestStatusBatched`.

## Test Plan

✅ `make test` — all unit tests pass; no remaining `BatchStateScored` /
`RequestStatusScored` references.
Not run locally: integration and e2e (need Docker), and
`//submitqueue/extension/pusher/git` (pre-existing macOS OpenSSL link
issue unrelated to this change) — CI covers these on Linux.
JamyDev pushed a commit that referenced this pull request Jul 24, 2026
## Summary

### Why?

Scoring is no longer a standalone pipeline stage — it is becoming a
speculator concern. The dedicated `score` stage between `batch` and
`speculate` added a queue hop and a batch state transition (`Created` →
`Scored`) without owning a distinct decision, so it is redundant.
Removing it lets `batch` hand batches straight to `speculate`.

### What?

The `batch` controller now publishes to `TopicKeySpeculate` instead of
`TopicKeyScore`. `speculate` already accepts `BatchStateCreated`, so
batches flow through without the intermediate `Scored` transition and
speculate itself needs no change.

Removed: the `score` controller package, `TopicKeyScore`, the score
topic and DLQ registrations, and the orchestrator's `scorerFactory`
wiring. Also removed the batch-level score data — the `Batch.Score`
field, the batch store's `UpdateScoreAndState` method (no remaining
caller), and the `score` column from the batch schema and queries.

Left intact: the `scorer` extension (all impls + mock) and the per-queue
scorer profiles, retained for the forthcoming scorer-in-speculate
integration. The `BatchStateScored` and `RequestStatusScored` enums
remain — `BatchStateScored` is still accepted by the untouched speculate
controller.

Docs and stale controller-list comments were updated to drop the `score`
stage; scorer-extension references are preserved.

## Test Plan

✅ `make test` — 79/79 unit tests pass (including the retained scorer
tests and the reworked mysql batch store test)
✅ `bazel build` of the orchestrator server — compiles clean, no unused
wiring
✅ `make gazelle` in sync
Not run: `make e2e-test` (needs Docker). The e2e happy-path assertion
was updated to drop `RequestStatusScored`, since requests no longer pass
through the scored status.
JamyDev pushed a commit that referenced this pull request Jul 24, 2026
## Summary

### Why?

The dedicated score stage was removed in #435, so nothing transitions a
batch into `BatchStateScored` or emits the `RequestStatusScored`
request-log status anymore. They are now dead, unreachable states — the
speculate controller only kept accepting `BatchStateScored` defensively
for a producer that no longer exists.

### What?

Removed the `BatchStateScored` and `RequestStatusScored` enum values.

- `entity/batch.go`: dropped the const and its entries in
`ActiveBatchStates()` and `DependencyBatchStates()`.
- `entity/request_log.go`: dropped the `RequestStatusScored` const.
- `speculate` controller: `startSpeculation` now switches on
`BatchStateCreated` only (dropped the unreachable `BatchStateScored`
case); doc comment updated to match.
- Tests: dropped the speculate `from_scored` case and switched the
`RequestStatusScored` sample-data usages in the request-log tests to
`RequestStatusBatched`.

## Test Plan

✅ `make test` — all unit tests pass; no remaining `BatchStateScored` /
`RequestStatusScored` references.
Not run locally: integration and e2e (need Docker), and
`//submitqueue/extension/pusher/git` (pre-existing macOS OpenSSL link
issue unrelated to this change) — CI covers these on Linux.
JamyDev pushed a commit that referenced this pull request Jul 24, 2026
## Summary

### Why?

Scoring is no longer a standalone pipeline stage — it is becoming a
speculator concern. The dedicated `score` stage between `batch` and
`speculate` added a queue hop and a batch state transition (`Created` →
`Scored`) without owning a distinct decision, so it is redundant.
Removing it lets `batch` hand batches straight to `speculate`.

### What?

The `batch` controller now publishes to `TopicKeySpeculate` instead of
`TopicKeyScore`. `speculate` already accepts `BatchStateCreated`, so
batches flow through without the intermediate `Scored` transition and
speculate itself needs no change.

Removed: the `score` controller package, `TopicKeyScore`, the score
topic and DLQ registrations, and the orchestrator's `scorerFactory`
wiring. Also removed the batch-level score data — the `Batch.Score`
field, the batch store's `UpdateScoreAndState` method (no remaining
caller), and the `score` column from the batch schema and queries.

Left intact: the `scorer` extension (all impls + mock) and the per-queue
scorer profiles, retained for the forthcoming scorer-in-speculate
integration. The `BatchStateScored` and `RequestStatusScored` enums
remain — `BatchStateScored` is still accepted by the untouched speculate
controller.

Docs and stale controller-list comments were updated to drop the `score`
stage; scorer-extension references are preserved.

## Test Plan

✅ `make test` — 79/79 unit tests pass (including the retained scorer
tests and the reworked mysql batch store test)
✅ `bazel build` of the orchestrator server — compiles clean, no unused
wiring
✅ `make gazelle` in sync
Not run: `make e2e-test` (needs Docker). The e2e happy-path assertion
was updated to drop `RequestStatusScored`, since requests no longer pass
through the scored status.
JamyDev pushed a commit that referenced this pull request Jul 24, 2026
## Summary

### Why?

The dedicated score stage was removed in #435, so nothing transitions a
batch into `BatchStateScored` or emits the `RequestStatusScored`
request-log status anymore. They are now dead, unreachable states — the
speculate controller only kept accepting `BatchStateScored` defensively
for a producer that no longer exists.

### What?

Removed the `BatchStateScored` and `RequestStatusScored` enum values.

- `entity/batch.go`: dropped the const and its entries in
`ActiveBatchStates()` and `DependencyBatchStates()`.
- `entity/request_log.go`: dropped the `RequestStatusScored` const.
- `speculate` controller: `startSpeculation` now switches on
`BatchStateCreated` only (dropped the unreachable `BatchStateScored`
case); doc comment updated to match.
- Tests: dropped the speculate `from_scored` case and switched the
`RequestStatusScored` sample-data usages in the request-log tests to
`RequestStatusBatched`.

## Test Plan

✅ `make test` — all unit tests pass; no remaining `BatchStateScored` /
`RequestStatusScored` references.
Not run locally: integration and e2e (need Docker), and
`//submitqueue/extension/pusher/git` (pre-existing macOS OpenSSL link
issue unrelated to this change) — CI covers these on Linux.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants